Skip to content

Conversation

@DmitryBochkarev
Copy link

@DmitryBochkarev DmitryBochkarev commented Dec 16, 2016

Without this patch method find_last_newline returns value bigger than
find_next_newline and in line original_line.length = line_end - line_start;
overflow happens.

Before changes newly added test failed with segmentation failure:

./test-driver: line 107: 12171 Segmentation fault      (core dumped)
"$@" > $log_file 2>&1

This slightly changed copy of code used in nokogumbo gem.
Link

https://jira.railsc.ru/browse/SERVICES-1513

Суть такая: ошибка парсинга возникает на символе окончания строки, функция find_last_newline должна вернуть начало строки, до места ошибки, но поскольку ошибка возникает на символе переноса, то возвращается позиция за местом ошибки, что противоречит назначению функции.

реквест в основной репозиторий google#371
там есть проблема, что там у основного майнтейнера отобрали права, а больше никто этой либой из гугла похоже не занимается google#370 (comment)

Without this patch method find_last_newline returns value bigger than
find_next_newline and in line `original_line.length = line_end -
line_start;`
overflow happens.

Before changes newly added test failed with segmentation failure:
```
./test-driver: line 107: 12171 Segmentation fault      (core dumped)
"$@" > $log_file 2>&1
```

This slightly changed copy of code used in nokogumbo gem.
[Link](https://github.com/rubys/nokogumbo/blob/8b4446847dea5c614759684ebcae4c580c47f4ad/ext/nokogumboc/nokogumbo.c#L230)
@DmitryBochkarev
Copy link
Author

@deniskorobicyn @Napolskih не знаю кого еще можно позвать

@Napolskih
Copy link

@deniskorobicyn
@c1n1c

Copy link

@deniskorobicyn deniskorobicyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Интересно, главное у них есть assert выше, но почему-то только на EOF, поидее это же можно до цикла сделать сразу

assert ( '\n' == *error_location);

Хотя хз че за assert у них, может свой с блекджеком, короч как у тебя тоже пойдет 👍

@DmitryBochkarev
Copy link
Author

У меня есть подозрение, что этот ассерт никогда не выполняется, наверное добавили, когда писали, так и оставили. Плохо что на error.c вообще нету никаких тестов, он походу используется только при написании биндингов.

@c1n1c c1n1c merged commit 8fcbc03 into abak-press:master Dec 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants